UFAL/Fix: generate correct curl download URLs using backend handle endpoint#1215
Open
milanmajchrak wants to merge 12 commits intodtq-devfrom
Open
UFAL/Fix: generate correct curl download URLs using backend handle endpoint#1215milanmajchrak wants to merge 12 commits intodtq-devfrom
milanmajchrak wants to merge 12 commits intodtq-devfrom
Conversation
Updates the curl command generation to use the new backend endpoint
GET /api/core/bitstreams/handle/{prefix}/{suffix}/{filename} instead
of the non-existent /api/bitstream/{handle}/{seq}/{filename}.
Key changes:
- Uses correct backend endpoint path: /core/bitstreams/handle/{handle}/
- Removes unnecessary sequence index from URLs (uses filename only)
- Quotes the URL to prevent shell brace expansion
- For single file, uses -o with explicit filename
Fixes: #1210
…rl -O curl -O uses the URL path as the saved filename, so percent-encoded characters (e.g. %20, %2B, %28) stay encoded in the output file. Now generates separate 'curl -o realname url' for each file joined with &&, ensuring files are saved with their actual names.
There was a problem hiding this comment.
Pull request overview
This pull request fixes the curl command generation for downloading bitstreams from DSpace items. The previous implementation referenced a non-existent backend endpoint /api/bitstream/{handle}/{seq}/{filename}, which has been replaced with the correct endpoint /api/core/bitstreams/handle/{prefix}/{suffix}/{filename}. The fix enables users to download files via command line using proper curl commands.
Changes:
- Updated endpoint path from
/bitstream/to/core/bitstreams/handle/ - Removed sequence index from URL construction (now uses filename only)
- Added URL quoting to prevent shell brace expansion
- Added
-Jflag to curl command to use Content-Disposition header filename - Introduced
encodeFilenameForUrl()method for proper filename URL encoding
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Outdated
Show resolved
Hide resolved
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Outdated
Show resolved
Hide resolved
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Outdated
Show resolved
Hide resolved
src/app/item-page/clarin-files-section/clarin-files-section.component.ts
Show resolved
Hide resolved
Collaborator
|
@milanmajchrak, you have two unresolved Copilot conversations. Have you already implemented its suggestions? |
Collaborator
|
Used endpoint: dataquest-dev/DSpace#1252 |
curl -J (Content-Disposition) cannot create files with non-ASCII characters on Windows because it interprets the header bytes using the console code page. Changed to curl -o filename url format where the shell passes the filename directly to the OS, correctly handling Unicode on all platforms. Also added tests for UTF-8 filenames and double-quote escaping.
encodeRFC3986URIComponent calls decodeURIComponent first, which throws URIError on filenames containing a literal percent sign (e.g. '100% done.txt') because '%' followed by non-hex chars is not a valid escape sequence. Replaced with inline encodeURIComponent() + parentheses encoding directly on the raw filename. Added test for literal percent sign in filenames.
curl command now uses brace expansion for compact URL:
curl -o file1 -o file2 baseUrl{/encoded1,/encoded2}
This combines:
- {} brace expansion in the URL (compact, one URL for all files)
- -o flags with real filenames (handles UTF-8 correctly via shell)
…ren) New FE test for 'Media (+)#9) ano' verifying correct URL encoding in brace expansion and real filename in -o flag.
curl URL globbing ({}) does NOT support per-file -o flags. When using
curl -o f1 -o f2 url{/a,/b}
curl maps the -o flags to URL arguments, not to globbed expansions,
resulting in 'Got more output options than URLs' and only one file saved.
Changed to separate -o + URL pairs per file:
curl -o file1 url/file1 -o file2 url/file2
Updated all 12 test expectations to match.
Replace inline command display with a centered NgbModal (size: lg) that shows the curl command in a scrollable pre block. Includes a copy-to-clipboard button with visual feedback (checkmark + 'Copied!' for 2s). - Added NgbModal injection and openCommandModal()/copyCommand() methods - Removed old isCommandLineVisible toggle and #command-div hover styles - Added i18n keys for en, cs, de (copy/copied/close) - Updated spec to import NgbModalModule
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem description
Updates the curl command generation to use the new backend endpoint GET /api/core/bitstreams/handle/{prefix}/{suffix}/{filename} instead of the non-existent /api/bitstream/{handle}/{seq}/{filename}.
Key changes:
Fixes: #1210
Sync verification
If en.json5 or cs.json5 translation files were updated:
yarn run sync-i18n -t src/assets/i18n/cs.json5 -ito synchronize messages, and changes are included in this PR.Manual Testing (if applicable)
Copilot review